Skip to content

feat: member management dashboard — profiles, stats, XP, warnings#119

Merged
BillChirico merged 31 commits intomainfrom
feat/member-management
Feb 28, 2026
Merged

feat: member management dashboard — profiles, stats, XP, warnings#119
BillChirico merged 31 commits intomainfrom
feat/member-management

Conversation

@BillChirico
Copy link
Collaborator

Summary

Full member management system for the web dashboard.

Backend API (src/api/routes/members.js)

  • GET /:id/members — Enhanced member list with stats/XP/warnings enrichment, search, sort, cursor pagination
  • GET /:id/members/export — CSV export of all members with stats
  • GET /:id/members/:userId — Full member profile (Discord info + user_stats + reputation + warnings)
  • GET /:id/members/:userId/cases — Paginated moderation case history
  • POST /:id/members/:userId/xp — Admin XP adjustment with level recomputation

Dashboard UI

  • Members list page — Searchable/sortable table with avatars, XP badges, warning counts, relative timestamps, cursor pagination
  • Member detail page — Full profile with stats cards, warning history, XP adjustment form, CSV export
  • Responsive — Mobile-friendly (hides secondary columns)

Tests

  • 26 new backend API tests
  • All 2226 tests passing, 0 regressions

Closes #32

Copilot AI review requested due to automatic review settings February 28, 2026 01:57
@claude
Copy link

claude bot commented Feb 28, 2026

Claude encountered an error —— View job


Reviewing PR #119

  • Read AGENTS.md and CLAUDE.md for project conventions
  • Review backend API (src/api/routes/members.js)
  • Review API mounting (src/api/index.js)
  • Review frontend pages and components
  • Review Next.js proxy routes
  • Review test files
  • Post inline comments for issues found
  • Submit review verdict

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 28, 2026

Warning

Rate limit exceeded

@BillChirico has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 7 minutes and 4 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 01e0cbc and 0350a59.

📒 Files selected for processing (6)
  • src/api/routes/members.js
  • tests/api/routes/members.test.js
  • tests/modules/rateLimit.test.js
  • web/src/app/dashboard/members/[userId]/page.tsx
  • web/src/app/dashboard/members/page.tsx
  • web/src/components/dashboard/member-table.tsx
📝 Walkthrough

Walkthrough

Adds a dedicated members router and Next.js proxy routes; removes the old GET /:id/members from guilds, exposes guild validation/admin helpers, implements enriched member list, CSV export, member detail, cases, and XP adjustment endpoints, and adds frontend pages/components and comprehensive tests.

Changes

Cohort / File(s) Summary
API entry & routing
src/api/index.js
Imports and mounts new members router (with requireAuth) before existing /guilds routes so /:id/members/* is handled first.
Guilds route updates
src/api/routes/guilds.js
Removed GET /:id/members; made requireGuildAdmin and validateGuild exported for reuse.
Members router (new)
src/api/routes/members.js
New default-exported Express router with global rate limit and routes: GET list (cursor pagination, search, sort), GET export (CSV streaming), GET member detail, GET member cases, POST XP adjustments; includes batched DB enrichment, validation, CSV escaping, and logging.
Backend tests
tests/api/routes/guilds.test.js, tests/api/routes/members.test.js
Removed old guild-members tests; added comprehensive members route tests covering list/detail/cases/xp/export, auth, validation, pagination, and DB/error scenarios.
Web dashboard pages
web/src/app/dashboard/members/page.tsx, web/src/app/dashboard/members/[userId]/page.tsx
Added Members list and Member detail pages with search/sort/pagination, profile/stats/XP UI, warnings table, admin XP adjustment form, CSV export action, and client-side state/UX handling.
Frontend component
web/src/components/dashboard/member-table.tsx
New MemberTable component and exported types (MemberRow, SortColumn, SortOrder) with sortable headers, accessibility, loading/empty states, and load-more support.
Web API proxy routes
web/src/app/api/guilds/[guildId]/members/route.ts, .../export/route.ts, .../[userId]/route.ts, .../[userId]/xp/route.ts, .../[userId]/cases/route.ts
New Next.js App Router handlers that validate params, authorize guild admins, build upstream URLs, proxy list/export/detail/xp/cases to the bot API, and stream/export CSVs; use dynamic = 'force-dynamic' and consistent log prefixes.
Modules & tests elsewhere
tests/modules/events-extra.test.js, tests/modules/rateLimit.test.js, tests/sentry.init.test.js
Added/expanded test suites (events, rateLimit cleanup export, Sentry init spy-based tests) to cover new behavior and new exported stopRateLimitCleanup.

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 77.42% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly and specifically describes the main change: adding a full member management dashboard with profiles, stats, XP, and warnings capabilities.
Description check ✅ Passed The PR description comprehensively outlines the backend API endpoints, dashboard UI components, and test coverage, directly relating to the implemented member management system.
Linked Issues check ✅ Passed The PR implements most core requirements from issue #32: member table with search/sort/pagination, detail view with profile/stats/warnings, XP adjustment, paginated case history, and CSV export. Memory management, conversation history, and warning issuance remain outstanding but are noted as intentional scope exclusions.
Out of Scope Changes check ✅ Passed All changes directly support the member management feature. Backend API routes, frontend pages, components, tests, and utility modifications align with stated objectives. Changes to guilds.js (exporting requireGuildAdmin, removing old endpoint) are necessary refactoring to support the new members architecture.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/member-management

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.


// Member management routes — require API secret or OAuth2 JWT
// (mounted before guilds to handle /:id/members/* before the basic guilds endpoint)
router.use('/guilds', requireAuth(), membersRouter);

Check failure

Code scanning / CodeQL

Missing rate limiting High

This route handler performs
authorization
, but is not rate-limited.
This route handler performs
authorization
, but is not rate-limited.

Copilot Autofix

AI 15 days ago

In general terms, we should introduce an Express rate-limiting middleware (for example, via express-rate-limit) and apply it to the sensitive routes that perform authorization and likely touch expensive resources. This middleware should run before requireAuth() (or at least before the main route handlers) so that abusive clients get rejected early. We can scope the limiter to specific routes (/guilds, /config, /moderation, /webhooks) instead of globally, to avoid impacting public endpoints like /health and /auth.

For this file specifically (src/api/index.js), the best fix is:

  • Import express-rate-limit.
  • Define a limiter instance configured with a reasonable window and max request count (for example, 15 minutes / 100 requests, similar to the example).
  • Apply that limiter middleware on the protected route groups, by inserting it into the router.use chains before requireAuth().

Concretely:

  • At the top of src/api/index.js, add import rateLimit from 'express-rate-limit';.

  • Under const router = Router();, define a constant such as const authRateLimiter = rateLimit({ windowMs: 15 * 60 * 1000, max: 100 });.

  • Update lines 25, 29, 32, 35, and 38 so they become:

    router.use('/config', authRateLimiter, requireAuth(), configRouter);
    router.use('/guilds', authRateLimiter, requireAuth(), membersRouter);
    router.use('/guilds', authRateLimiter, requireAuth(), guildsRouter);
    router.use('/moderation', authRateLimiter, requireAuth(), moderationRouter);
    router.use('/webhooks', authRateLimiter, requireAuth(), webhooksRouter);

This way, all routes that rely on requireAuth() and likely do expensive authorization work are now rate-limited, addressing all alert variants, while not changing existing functional behavior beyond adding throttling on abusive patterns.

Suggested changeset 1
src/api/index.js

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/api/index.js b/src/api/index.js
--- a/src/api/index.js
+++ b/src/api/index.js
@@ -4,6 +4,7 @@
  */
 
 import { Router } from 'express';
+import rateLimit from 'express-rate-limit';
 import { requireAuth } from './middleware/auth.js';
 import authRouter from './routes/auth.js';
 import configRouter from './routes/config.js';
@@ -15,6 +16,11 @@
 
 const router = Router();
 
+const authRateLimiter = rateLimit({
+  windowMs: 15 * 60 * 1000, // 15 minutes
+  max: 100, // limit each IP to 100 authenticated requests per windowMs
+});
+
 // Health check — public (no auth required)
 router.use('/health', healthRouter);
 
@@ -22,19 +28,19 @@
 router.use('/auth', authRouter);
 
 // Global config routes — require API secret or OAuth2 JWT
-router.use('/config', requireAuth(), configRouter);
+router.use('/config', authRateLimiter, requireAuth(), configRouter);
 
 // Member management routes — require API secret or OAuth2 JWT
 // (mounted before guilds to handle /:id/members/* before the basic guilds endpoint)
-router.use('/guilds', requireAuth(), membersRouter);
+router.use('/guilds', authRateLimiter, requireAuth(), membersRouter);
 
 // Guild routes — require API secret or OAuth2 JWT
-router.use('/guilds', requireAuth(), guildsRouter);
+router.use('/guilds', authRateLimiter, requireAuth(), guildsRouter);
 
 // Moderation routes — require API secret or OAuth2 JWT
-router.use('/moderation', requireAuth(), moderationRouter);
+router.use('/moderation', authRateLimiter, requireAuth(), moderationRouter);
 
 // Webhook routes — require API secret or OAuth2 JWT (endpoint further restricts to api-secret)
-router.use('/webhooks', requireAuth(), webhooksRouter);
+router.use('/webhooks', authRateLimiter, requireAuth(), webhooksRouter);
 
 export default router;
EOF
@@ -4,6 +4,7 @@
*/

import { Router } from 'express';
import rateLimit from 'express-rate-limit';
import { requireAuth } from './middleware/auth.js';
import authRouter from './routes/auth.js';
import configRouter from './routes/config.js';
@@ -15,6 +16,11 @@

const router = Router();

const authRateLimiter = rateLimit({
windowMs: 15 * 60 * 1000, // 15 minutes
max: 100, // limit each IP to 100 authenticated requests per windowMs
});

// Health check — public (no auth required)
router.use('/health', healthRouter);

@@ -22,19 +28,19 @@
router.use('/auth', authRouter);

// Global config routes — require API secret or OAuth2 JWT
router.use('/config', requireAuth(), configRouter);
router.use('/config', authRateLimiter, requireAuth(), configRouter);

// Member management routes — require API secret or OAuth2 JWT
// (mounted before guilds to handle /:id/members/* before the basic guilds endpoint)
router.use('/guilds', requireAuth(), membersRouter);
router.use('/guilds', authRateLimiter, requireAuth(), membersRouter);

// Guild routes — require API secret or OAuth2 JWT
router.use('/guilds', requireAuth(), guildsRouter);
router.use('/guilds', authRateLimiter, requireAuth(), guildsRouter);

// Moderation routes — require API secret or OAuth2 JWT
router.use('/moderation', requireAuth(), moderationRouter);
router.use('/moderation', authRateLimiter, requireAuth(), moderationRouter);

// Webhook routes — require API secret or OAuth2 JWT (endpoint further restricts to api-secret)
router.use('/webhooks', requireAuth(), webhooksRouter);
router.use('/webhooks', authRateLimiter, requireAuth(), webhooksRouter);

export default router;
Copilot is powered by AI and may make mistakes. Always verify output.
@greptile-apps
Copy link

greptile-apps bot commented Feb 28, 2026

Greptile Summary

Implements a comprehensive member management dashboard with backend APIs for enriched member data (stats, XP, warnings), frontend UI for browsing and managing members, and full test coverage.

Backend changes:

  • New /guilds/:id/members API routes with Discord data enrichment from PostgreSQL (stats, XP, warnings)
  • Supports search, sorting (messages/XP/warnings/joined), cursor pagination, and CSV export
  • XP adjustment endpoint with transaction safety and level recomputation
  • Moved member routes from guilds.js to dedicated members.js router
  • Exported middleware (requireGuildAdmin, validateGuild) for reuse

Frontend changes:

  • Members list page with real-time search (300ms debounce), sortable columns, infinite scroll pagination
  • Member detail page with stats cards, XP progress bars, warning history, admin XP adjustment form
  • Next.js API proxy routes for all member endpoints with proper auth and error handling
  • Responsive table component with keyboard navigation, loading states, empty states
  • AbortController + request sequencing to prevent race conditions from stale responses

Tests:

  • 26 new backend API tests covering all endpoints, edge cases, auth, pagination, errors
  • All 2226 tests passing with zero regressions

Issues addressed from previous comments:

  • API proxy routes are now implemented (were flagged as missing)
  • CSV export streams in batches to reduce memory pressure (1000 members per batch)

Confidence Score: 4/5

  • Safe to merge with one minor performance optimization and addressing previously flagged issues
  • Well-structured implementation with comprehensive tests (26 new tests, all passing), proper error handling, security (rate limiting, auth checks, SQL parameterization, CSV injection protection), and good UX patterns (AbortController, request sequencing, debouncing). One style issue with redundant array creation. Previous reviewers flagged CSV injection bug and missing next_level_xp in XP response — these should be verified/addressed.
  • Review src/api/routes/members.js for previously flagged CSV injection issue and missing next_level_xp field in XP adjustment response

Important Files Changed

Filename Overview
src/api/routes/members.js New member management API with enriched data, XP adjustment, CSV export; one minor performance issue with redundant array creation
web/src/app/dashboard/members/page.tsx Members list page with search, sorting, cursor pagination, AbortController, and request sequencing
web/src/app/dashboard/members/[userId]/page.tsx Member detail page with stats cards, warning history, XP adjustment form, and CSV export
web/src/components/dashboard/member-table.tsx Sortable, responsive member table with avatars, XP badges, warnings, relative timestamps, keyboard navigation
tests/api/routes/members.test.js Comprehensive test suite with 26 tests covering all member endpoints, edge cases, and error conditions

Sequence Diagram

sequenceDiagram
    participant UI as Dashboard UI
    participant Proxy as Next.js Proxy
    participant API as Bot API
    participant Discord as Discord API
    participant DB as PostgreSQL

    Note over UI,DB: Member List Flow
    UI->>Proxy: GET /api/guilds/{id}/members?search=alice&sort=xp
    Proxy->>API: GET /guilds/{id}/members (with auth)
    API->>Discord: members.search() or members.list()
    Discord-->>API: Collection of GuildMembers
    
    par Enrich member data
        API->>DB: SELECT user_stats WHERE user_id IN (...)
        DB-->>API: messages, days_active, last_active
        API->>DB: SELECT reputation WHERE user_id IN (...)
        DB-->>API: xp, level
        API->>DB: SELECT COUNT(*) FROM mod_cases (warnings)
        DB-->>API: warning counts
    end
    
    API->>API: Merge Discord + DB data, apply sort
    API-->>Proxy: {members, nextAfter, total}
    Proxy-->>UI: JSON response
    UI->>UI: Render table with avatars, XP badges, warnings

    Note over UI,DB: XP Adjustment Flow
    UI->>Proxy: POST /api/.../members/{userId}/xp<br/>{amount: 100, reason: "..."}
    Proxy->>API: Forward POST with auth
    API->>DB: BEGIN TRANSACTION
    API->>DB: UPSERT reputation SET xp = xp + amount
    DB-->>API: {xp, level}
    API->>API: computeLevel(xp, thresholds)
    API->>DB: UPDATE reputation SET level = newLevel
    API->>DB: COMMIT
    DB-->>API: Success
    API-->>Proxy: {userId, xp, level, adjustment}
    Proxy-->>UI: JSON response
    UI->>UI: Update stats cards without reload
Loading

Last reviewed commit: 0350a59

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/api/routes/members.js`:
- Around line 38-50: The CSV export currently uses guild.members.list({ limit:
1000 }) in the route handler for '/:id/members/export' which will miss members
on large guilds; update the export logic to paginate through all members by
repeatedly calling guild.members.list with an after cursor (e.g., lastId) and a
PAGE_SIZE (1000) until no more results, accumulating members into allMembers
(Map or array), then proceed using that collection instead of the single
`members` batch; ensure you update places that reference `members` to use the
accumulated collection and handle memory/streaming for very large exports or
fall back to documenting the 1000-member limitation if pagination is not
implemented.
- Around line 335-339: The roles array returned from the member serialization
uses r.hexColor (a string) but the frontend MemberDetail expects color as a
number; update the roles mapping in the member response (the block building
roles from member.roles.cache in the route that currently maps id/name/color) to
return color as an integer (e.g., convert the hex string to a numeric value) so
it matches the frontend roleColor helper/MemberDetail shape; ensure the property
name remains color and not hexColor and reference the roles mapping logic in the
member serialization code when implementing the conversion.

In `@tests/api/routes/members.test.js`:
- Around line 128-159: The API returns members with the field messages_sent but
the frontend MemberRow interface expects messages; update the frontend type to
match the backend by renaming the MemberRow property from messages to
messages_sent (or add messages_sent as an alias) so components reading MemberRow
(member-table.tsx's MemberRow interface and any usages) access messages_sent;
alternatively, if you prefer changing the backend, update the members API
response code that constructs member objects (the members handler) to emit
messages instead of messages_sent so the MemberRow interface remains unchanged.

In `@web/src/app/dashboard/members/`[userId]/page.tsx:
- Around line 258-277: The export handler handleExport currently swallows
errors; update its catch block to capture the thrown error (e.g., catch (err))
and surface feedback to the user instead of failing silently: call the app's
toast/error UI (or set a local error state) to show a meaningful message like
"Export failed" plus the error message, and also log the error to console or
monitoring for debugging; keep setExporting(false) in finally so UI state is
consistent. Use the existing handleExport, setExporting, and the fetch response
logic as the insertion points for the toast/log calls.
- Around line 37-54: The MemberDetail interface fields don't match the API
response; update MemberDetail (or map the API response) so its properties match
the API's camelCase names and types: use id (string) instead of user_id,
displayName (string|null) instead of display_name, avatar (string|null) instead
of avatar_hash (and treat it as a full URL), joinedAt and lastActive
(string|null) instead of joined_at/last_active, and change roles[].color to
roles[].hexColor (string) or convert the hex string to a number when mapping;
ensure any consumers of MemberDetail (e.g., code referencing MemberDetail in
page.tsx) are updated to use the new property names or that you add a mapper
function to transform the API response to the existing interface.

In `@web/src/app/dashboard/members/page.tsx`:
- Around line 159-164: handleRowClick currently calls
router.push(`/dashboard/members/${userId}`) which loses guild context; change it
to include the selected guild ID (from useGuildSelection() or the component
prop, e.g. router.push(`/dashboard/guilds/${guildId}/members/${userId}`)) so
member detail pages retain guild context on refresh/new tab, and update the
useCallback dependency array to include guildId (and any selection hook) so the
route updates correctly; ensure the member detail route and any consumers of
useGuildSelection expect the new URL shape.
- Around line 106-117: The effect using useEffect omits fetchMembers from its
dependency array via an eslint-disable, risking a stale closure; include
fetchMembers in the dependency list of the effect that calls it (the useEffect
block invoking fetchMembers) since fetchMembers is defined with useCallback
(dependent on onUnauthorized), which prevents re-creation loops—ensure
fetchMembers is referenced instead of suppressing react-hooks/exhaustive-deps so
the effect depends on [guildId, debouncedSearch, sortColumn, sortOrder,
fetchMembers].

In `@web/src/components/dashboard/member-table.tsx`:
- Around line 251-319: Update the member row rendering to match the revised
MemberRow interface: replace m.user_id with m.id for keys and onRowClick, adapt
avatarUrl calls to use m.id and m.avatarHash (or the new avatar field names from
the interface) in the Image src/alt logic, rename m.display_name to
m.displayName where used, change m.messages to m.messages_sent in the
formatNumber call, change m.warnings to m.warning_count for the badge logic, and
change m.joined_at to m.joinedAt for formatDateShort; ensure all references to
these fields within the members.map callback (variable m) and any helper calls
(avatarUrl, formatNumber, relativeTime, formatDateShort) are updated to the new
property names.
- Around line 33-41: The UI SortColumn type includes values that the backend
doesn't support, causing silent fallback; update the SortColumn in
member-table.tsx to only include the API-supported keys ('messages', 'xp',
'warnings', 'joined') or add a mapping layer that converts UI column names
(e.g., 'joined_at', 'username', 'display_name', 'level', 'last_active') to those
API params before calling the members API; specifically edit the SortColumn
union and any code that reads SortColumn (sorting handlers/renderers) to either
remove unsupported options or map them to the API keys so requests only use
'messages', 'xp', 'warnings', or 'joined'.
- Around line 20-31: The MemberRow interface fields don't match the API's GET
/:id/members response; update the MemberRow type (used by the member-table
component) to use the API field names (rename user_id → id, avatar_hash →
avatar, messages → messages_sent, warnings → warning_count, and keep/adjust
other fields as necessary), then update all places in this file that reference
those properties (e.g., change m.user_id to m.id, m.messages to m.messages_sent,
m.avatar_hash to m.avatar, m.warnings to m.warning_count) so the table reads the
correct values returned by the API.
- Around line 58-61: The avatarUrl helper (function avatarUrl) currently treats
its second argument as a Discord hash and builds a CDN path, but the API now
supplies member.user.displayAvatarURL() as a full URL; change avatarUrl to
accept and return a full URL (or detect if the passed value already starts with
"http" and return it unchanged) and update the call site in MemberTable where
avatarUrl(...) is invoked to pass the API-provided avatar string directly rather
than a hash so that malformed CDN URLs are no longer produced.

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 27112e7 and 6b8547f.

⛔ Files ignored due to path filters (2)
  • package-lock.json is excluded by !**/package-lock.json
  • web/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (8)
  • src/api/index.js
  • src/api/routes/guilds.js
  • src/api/routes/members.js
  • tests/api/routes/guilds.test.js
  • tests/api/routes/members.test.js
  • web/src/app/dashboard/members/[userId]/page.tsx
  • web/src/app/dashboard/members/page.tsx
  • web/src/components/dashboard/member-table.tsx
💤 Files with no reviewable changes (1)
  • tests/api/routes/guilds.test.js
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Greptile Review
  • GitHub Check: Agent
  • GitHub Check: claude-review
🧰 Additional context used
📓 Path-based instructions (5)
**/*.js

📄 CodeRabbit inference engine (AGENTS.md)

**/*.js: Use ESM modules only — import/export, never require()
Always use node: protocol for Node.js builtin imports (e.g., import { readFileSync } from 'node:fs')

Files:

  • tests/api/routes/members.test.js
  • src/api/routes/members.js
  • src/api/routes/guilds.js
  • src/api/index.js
**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{js,ts,tsx}: Always use semicolons
Use single quotes — enforced by Biome
Use 2-space indentation — enforced by Biome

Files:

  • tests/api/routes/members.test.js
  • web/src/app/dashboard/members/page.tsx
  • web/src/app/dashboard/members/[userId]/page.tsx
  • web/src/components/dashboard/member-table.tsx
  • src/api/routes/members.js
  • src/api/routes/guilds.js
  • src/api/index.js
web/**/*.{tsx,ts}

📄 CodeRabbit inference engine (AGENTS.md)

Use next/image Image component with appropriate layout and sizing props in Next.js components

Files:

  • web/src/app/dashboard/members/page.tsx
  • web/src/app/dashboard/members/[userId]/page.tsx
  • web/src/components/dashboard/member-table.tsx
web/src/**/*.{tsx,ts}

📄 CodeRabbit inference engine (AGENTS.md)

Use Zustand store (zustand) for state management in React components; implement fetch-on-demand pattern in stores

Files:

  • web/src/app/dashboard/members/page.tsx
  • web/src/app/dashboard/members/[userId]/page.tsx
  • web/src/components/dashboard/member-table.tsx
src/**/*.js

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.js: NEVER use console.log, console.warn, console.error, or any console.* method in src/ files — always use Winston logger instead: import { info, warn, error } from '../logger.js'
Pass structured metadata to Winston logger: info('Message processed', { userId, channelId })
Use custom error classes from src/utils/errors.js and always log errors with context before re-throwing
Use getConfig(guildId?) from src/modules/config.js to read config; use setConfigValue(path, value, guildId?) to update at runtime
Use safeSend() utility for outgoing Discord messages to sanitize mentions and enforce allowedMentions
Use splitMessage() utility for messages exceeding Discord's 2000-character limit
Add tests for all new code with mandatory 80% coverage threshold on statements, branches, functions, and lines; run pnpm test before every commit

Files:

  • src/api/routes/members.js
  • src/api/routes/guilds.js
  • src/api/index.js
🧠 Learnings (1)
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to src/**/*.js : Use getConfig(guildId?) from src/modules/config.js to read config; use setConfigValue(path, value, guildId?) to update at runtime

Applied to files:

  • src/api/index.js
🧬 Code graph analysis (2)
tests/api/routes/members.test.js (2)
src/db.js (1)
  • getPool (142-147)
src/api/server.js (1)
  • createApp (27-87)
src/api/index.js (1)
src/api/middleware/auth.js (1)
  • requireAuth (36-70)
🪛 GitHub Check: CodeQL
src/api/routes/members.js

[failure] 41-41: Missing rate limiting
This route handler performs authorization, but is not rate-limited.


[failure] 43-124: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.


[failure] 138-138: Missing rate limiting
This route handler performs authorization, but is not rate-limited.


[failure] 138-263: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.


[failure] 273-273: Missing rate limiting
This route handler performs authorization, but is not rate-limited.


[failure] 275-373: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.


[failure] 387-387: Missing rate limiting
This route handler performs authorization, but is not rate-limited.


[failure] 389-433: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.


[failure] 446-446: Missing rate limiting
This route handler performs authorization, but is not rate-limited.


[failure] 448-510: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.

src/api/index.js

[failure] 29-29: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.

🔇 Additional comments (6)
src/api/routes/guilds.js (1)

268-293: LGTM — Clean export of shared middleware.

Exporting requireGuildAdmin and validateGuild enables proper reuse by the new members router while maintaining the authorization logic in one place. The removal of the old /:id/members endpoint aligns with the migration to the dedicated members router.

src/api/index.js (1)

27-29: LGTM — Correct router mounting order.

Mounting the members router before the general guilds router ensures that /guilds/:id/members/* paths are matched first. The rate limiting flagged by CodeQL is a false positive — rate limiting is correctly applied within membersRouter via the membersRateLimit middleware on each endpoint.

tests/api/routes/members.test.js (1)

45-116: LGTM — Well-structured test setup with comprehensive mocking.

The mock setup properly isolates the tests from external dependencies (logger, database, config, OAuth). The mock guild and member objects provide realistic Discord.js-like structures for testing.

web/src/app/dashboard/members/[userId]/page.tsx (1)

343-356: LGTM — Proper use of next/image with sizing props.

The Image component is correctly used with explicit width and height props as required by Next.js, and the avatar URL construction handles the null case appropriately.

src/api/routes/members.js (2)

17-20: LGTM — Rate limiting is correctly applied.

The CodeQL warnings are false positives. The membersRateLimit middleware is applied to all route handlers in this file (Lines 40, 138, 272, 386, 445), providing 120 requests per 15 minutes per IP protection for all database-accessing endpoints.


518-525: LGTM — Robust CSV escaping.

The escapeCsv helper correctly handles null values, commas, quotes (with proper doubling), and newlines per RFC 4180.

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

17 files reviewed, 15 comments

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new member management feature spanning the bot API and the web dashboard, enabling admins to browse members, inspect detailed profiles/case history, adjust XP, and export member data.

Changes:

  • Introduces new bot API member-management routes (list/export/detail/cases/XP adjust) and mounts them under /api/v1/guilds.
  • Adds new dashboard pages/components for the members list and member detail views.
  • Adds a dedicated test suite for the new members API and removes the older guild members tests.

Reviewed changes

Copilot reviewed 8 out of 10 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
web/src/components/dashboard/member-table.tsx New sortable/clickable members table UI component.
web/src/app/dashboard/members/page.tsx New members list page with search/sort/pagination state + fetching.
web/src/app/dashboard/members/[userId]/page.tsx New member detail page with stats, cases table, XP adjustment, CSV export.
tests/api/routes/members.test.js New Vitest coverage for members routes (list/detail/cases/XP/export).
tests/api/routes/guilds.test.js Removes legacy /members route tests from guilds suite.
src/api/routes/members.js New Express router implementing member management endpoints.
src/api/routes/guilds.js Exports requireGuildAdmin/validateGuild and removes old /members route.
src/api/index.js Mounts the new members router before the guilds router.
package-lock.json Lockfile updates for dependencies/engine alignment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…nsaction safety

- CSV injection: escapeCsv now prefixes formula chars (=,+,-,@,\t,\r) with single quote
- XP bounds: cap adjustments to ±1,000,000
- XP transaction: wrap upsert + level update in BEGIN/COMMIT/ROLLBACK
- Warning filter: add AND action = 'warn' to recent warnings query in member detail
- CSV export: paginate guild.members.list() for guilds > 1000 members
- Pool safety: wrap getPool() in safeGetPool() with try/catch, return 503 on failure
- Search total: return filteredTotal alongside total when search is active

Addresses review comments #8-15 on PR #119.
Frontend-backend contract fixes:
- MemberRow: id/displayName/avatar/messages_sent/warning_count/joinedAt (was snake_case)
- Avatar: use full URL from backend directly, remove hash-based CDN helper
- Pagination: cursor → nextAfter, param 'cursor' → 'after'
- MemberDetail: flat response shape with stats/reputation/warnings sub-objects
- SortColumn: restrict to API-supported values (messages/xp/warnings/joined)
- Role color: use hexColor string directly instead of number conversion
- XP progress: use next_level_xp from reputation object
- CSV export: add error state instead of silent catch
- Dependency array: add fetchMembers, remove eslint-disable comment
- Keyboard accessibility: tabIndex={0} + onKeyDown for Enter/Space on rows
- Guild context: handleRowClick depends on guildId
- Search total: display filteredTotal when available

Addresses review comments #1-7, #16-19 on PR #119.
Create proxy routes following the existing bot-api-proxy pattern:
- GET /api/guilds/:guildId/members — enriched member list
- GET /api/guilds/:guildId/members/:userId — member detail
- POST /api/guilds/:guildId/members/:userId/xp — XP adjustment
- GET /api/guilds/:guildId/members/:userId/cases — mod case history
- GET /api/guilds/:guildId/members/export — CSV export (streamed)

All routes include guild admin authorization and proper error handling.
CSV export uses 30s timeout and streams the response body.

Addresses review comment #5 on PR #119.
- XP tests: mock pool.connect() + client for transaction flow
- Add XP bounds test (±1,000,000 limit)
- Verify BEGIN/COMMIT/release called in transaction
- Search test: assert filteredTotal in response
- Use template literals instead of string concatenation (biome)
- Use const for non-reassigned variables
- Add button type='button' for a11y
- Remove unused displayTotal variable
- Use Number.isNaN over global isNaN
- Format proxy routes to match biome standards
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated 9 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/api/routes/members.test.js`:
- Around line 49-95: Create fresh per-test fixtures instead of sharing the
top-level mockGuild/mockMember1/mockMember2 objects: move the
mockMember1/mockMember2 and mockGuild creation into a beforeEach (or convert
them to factory functions like createMockMember and createMockGuild) so each
test gets new Maps and mocked methods. Ensure members.cache, members.list
(mockResolvedValue/ResolvedValueOnce) and members.fetch are newly created/reset
per test (or call vi.resetAllMocks() in beforeEach) to avoid cross-test mutation
when tests call mockResolvedValueOnce or mutate roles/channels.

In `@tests/modules/events-extra.test.js`:
- Around line 443-478: The tests call registerReadyHandler and invoke the ready
callback but never assert outcomes; update each case to (1) make the test async,
(2) capture the ready callback via client.once mock and await its invocation,
(3) spy on the logger used by registerReadyHandler (e.g.,
processLogger.info/console.info) to assert starboard logs contain the configured
channelId/threshold when config.starboard.enabled is true, and (4) for triage
model cases assert the resolved respond model was applied—e.g., check
config.triage.respondModel (or the module/state variable your handler sets)
equals 'gpt-4', 'custom-model', or 'explicit-model' respectively after running
the ready callback. Ensure each test restores/spies cleanup.

In `@tests/modules/rateLimit.test.js`:
- Around line 529-553: The test's final assertion only checks result.limited and
doesn't verify that the trigger counter was reset after the mute window; update
the test that calls checkRateLimit with the config to also assert the trigger
counter value (from the returned result) is 1 after the second trigger sequence
so it fails when the counter wasn't reset; locate the test block using the
makeConfig config, the msg variable and the calls to checkRateLimit and add an
assertion on result.triggerCount (and keep result.limited === true) to ensure
the trigger window was actually reset.
- Around line 582-624: The test for checkRateLimit currently only asserts
alertSend was called; update it to inspect the actual message payload passed to
alertSend (e.g., via alertSend.mock.calls) and assert the embed/content includes
the exact singular wording "1 minute" and does not contain "1 minutes" after
triggering the rate limit with muteWindowSeconds: 60; reference the test's
alertSend mock and the checkRateLimit invocation to locate where to add an
assertion on alertSend.mock.calls (or alertSend.mock.lastCall) for the embed or
content text.
- Around line 626-885: Remove the duplicated test suites that re-run scenarios
already defined earlier: delete the duplicate describe blocks named
"checkRateLimit — handleRepeatOffender edge cases", "checkRateLimit — trigger
window reset" (the second occurrence), the standalone "stopRateLimitCleanup"
duplicate, and "checkRateLimit — singular minute in alert" duplicate so only the
original suites remain; keep the first occurrences that reference
checkRateLimit, stopRateLimitCleanup, and related helper functions
(makeConfig/makeMessage) and then run the test suite to ensure no regressions.

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a121cb0 and 1a9f769.

📒 Files selected for processing (5)
  • src/api/index.js
  • tests/api/routes/members.test.js
  • tests/modules/events-extra.test.js
  • tests/modules/rateLimit.test.js
  • tests/sentry.init.test.js
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Agent
  • GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (3)
**/*.js

📄 CodeRabbit inference engine (AGENTS.md)

**/*.js: Use ESM modules only — import/export, never require()
Always use node: protocol for Node.js builtin imports (e.g., import { readFileSync } from 'node:fs')

Files:

  • tests/sentry.init.test.js
  • tests/modules/rateLimit.test.js
  • src/api/index.js
  • tests/modules/events-extra.test.js
  • tests/api/routes/members.test.js
**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{js,ts,tsx}: Always use semicolons
Use single quotes — enforced by Biome
Use 2-space indentation — enforced by Biome

Files:

  • tests/sentry.init.test.js
  • tests/modules/rateLimit.test.js
  • src/api/index.js
  • tests/modules/events-extra.test.js
  • tests/api/routes/members.test.js
src/**/*.js

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.js: NEVER use console.log, console.warn, console.error, or any console.* method in src/ files — always use Winston logger instead: import { info, warn, error } from '../logger.js'
Pass structured metadata to Winston logger: info('Message processed', { userId, channelId })
Use custom error classes from src/utils/errors.js and always log errors with context before re-throwing
Use getConfig(guildId?) from src/modules/config.js to read config; use setConfigValue(path, value, guildId?) to update at runtime
Use safeSend() utility for outgoing Discord messages to sanitize mentions and enforce allowedMentions
Use splitMessage() utility for messages exceeding Discord's 2000-character limit
Add tests for all new code with mandatory 80% coverage threshold on statements, branches, functions, and lines; run pnpm test before every commit

Files:

  • src/api/index.js
🧠 Learnings (3)
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to src/commands/*.js : Use parseDuration() from src/utils/duration.js for duration-based commands (timeout, tempban, slowmode); enforce Discord duration caps (timeouts max 28 days, slowmode max 6 hours)

Applied to files:

  • tests/modules/rateLimit.test.js
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to src/**/*.js : Use getConfig(guildId?) from src/modules/config.js to read config; use setConfigValue(path, value, guildId?) to update at runtime

Applied to files:

  • src/api/index.js
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to src/**/*.js : Add tests for all new code with mandatory 80% coverage threshold on statements, branches, functions, and lines; run pnpm test before every commit

Applied to files:

  • tests/modules/events-extra.test.js
🧬 Code graph analysis (3)
tests/modules/rateLimit.test.js (1)
src/modules/rateLimit.js (2)
  • checkRateLimit (146-230)
  • stopRateLimitCleanup (271-276)
src/api/index.js (8)
src/api/routes/members.js (1)
  • router (17-17)
src/api/routes/guilds.js (1)
  • router (21-21)
src/api/routes/moderation.js (1)
  • router (12-12)
src/api/routes/auth.js (1)
  • router (15-15)
src/api/routes/health.js (1)
  • router (28-28)
src/api/routes/webhooks.js (1)
  • router (12-12)
src/api/routes/config.js (1)
  • router (23-23)
src/api/middleware/auth.js (1)
  • requireAuth (36-70)
tests/api/routes/members.test.js (3)
src/api/routes/members.js (7)
  • req (306-306)
  • req (417-417)
  • req (473-473)
  • req (474-474)
  • member (316-316)
  • client (493-493)
  • client (499-506)
src/db.js (1)
  • getPool (142-147)
src/api/server.js (1)
  • createApp (27-87)
🪛 GitHub Check: CodeQL
src/api/index.js

[failure] 29-29: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.

🔇 Additional comments (9)
tests/sentry.init.test.js (6)

1-19: LGTM!

Clean mock setup that intercepts @sentry/node to capture the init configuration. The module-level spy allows inspection of the config passed to Sentry.init across all tests.


21-29: LGTM!

Proper test isolation using vi.resetModules() to ensure each dynamic import gets a fresh module evaluation, and vi.unstubAllEnvs() for cleanup.


33-72: LGTM!

Comprehensive coverage of the tracesSampleRate IIFE branches: valid numeric parsing, explicit zero, non-numeric fallback, and unset fallback. The use of delete process.env.SENTRY_TRACES_RATE correctly ensures the variable is absent for the fallback test.


76-106: LGTM!

Good coverage of the environment resolution chain: SENTRY_ENVIRONMENTNODE_ENV"production" fallback.


110-197: LGTM!

Thorough coverage of the beforeSend filter: AbortError filtering, sensitive key scrubbing (9 keys verified), preservation of safe fields, and graceful handling of events without extra context or exception values.


199-218: LGTM!

Good edge case coverage: events with empty exception values don't crash, and Sentry.init is correctly skipped when DSN is empty.

tests/modules/events-extra.test.js (1)

103-439: Strong branch/error-path coverage across interaction, message, and reaction handlers.

The suite does a good job exercising ignore paths, failure paths, and early returns with focused mocks and assertions.

Also applies to: 482-736

src/api/index.js (1)

27-32: Route ordering looks correct for members endpoints.

Mounting membersRouter before guildsRouter under /guilds is the right precedence to ensure /:id/members/* resolves to the specialized handlers first.

tests/api/routes/members.test.js (1)

674-754: Great CSV hardening coverage.

The escape-path tests for formula prefixes, commas, and quotes/newlines are solid regression protection for export safety.

claude[bot]
claude bot previously approved these changes Feb 28, 2026
Copy link

@claude claude bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

This PR implements a comprehensive member management dashboard and I'm impressed by the scope — backend API with enrichment, CSV export with formula injection protection, cursor-based pagination, XP transactions, Next.js proxy routes, and a full React UI. The code quality is high overall, especially after the many improvements made across previous review cycles.

Findings

🟡 Medium

  • handleAdjustXp and handleExport callbacks are missing router in their useCallback dependency arrays (page.tsx:306, page.tsx:333)

🔵 Low

What looks good

  • CSV formula injection protection with prefix approach
  • XP adjustment wrapped in a proper DB transaction with ROLLBACK
  • All 5 proxy routes implemented with consistent auth/config pattern
  • Parameterized SQL queries throughout (no injection risk)
  • Rate limiting scoped correctly to member routes only
  • Proper cursor-based pagination for Discord API
  • Good test coverage across all endpoints

The remaining issues are minor (no runtime bugs today) and safe to fix in a follow-up. Approving.

🤖 Prompt to fix remaining issues

Fix the following issues in PR #119 (feat/member-management):

  1. In web/src/app/dashboard/members/[userId]/page.tsx:

    • Line 306: Add router to the handleAdjustXp useCallback dependency array: [guildId, userId, xpAmount, xpReason, router]
    • Line 333: Add router to the handleExport useCallback dependency array: [guildId, router]
  2. In web/src/components/dashboard/member-table.tsx:

    • Line 130: Add type="button" to the sort header <button> element

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (12)
tests/api/routes/members.test.js (1)

49-95: 🧹 Nitpick | 🔵 Trivial

Consider isolating per-test fixtures to avoid cross-test state leakage.

mockGuild/members are shared across the suite and mutated in tests via mockResolvedValueOnce. Creating fresh fixture objects in beforeEach would make the suite more robust as coverage grows.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/api/routes/members.test.js` around lines 49 - 95, The shared mock
fixtures (mockGuild, mockMember1, mockMember2 and mockGuild.members with
members.list/members.fetch) are mutated across tests causing potential leakage;
move their construction into a beforeEach that creates fresh instances for
mockMember1, mockMember2 and mockGuild (including members.cache, members.list
and members.fetch mocks) so each test gets a new independent fixture, and update
tests that currently call mockResolvedValueOnce to operate on the per-test mocks
instead of the shared objects.
web/src/app/dashboard/members/[userId]/page.tsx (5)

204-205: ⚠️ Potential issue | 🟠 Major

Handle missing guild/user context to prevent infinite loading state.

When guildId or userId is missing, the effect returns early but loading remains true, causing the skeleton to display indefinitely.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/app/dashboard/members/`[userId]/page.tsx around lines 204 - 205, The
effect that fetches member data (useEffect) returns early when guildId or userId
is missing but never updates the loading state, leaving loading true
indefinitely; modify the effect in page.tsx to set loading to false (or call
setLoading(false)) and optionally set an error or null member when guildId or
userId is falsy before returning, so the skeleton stops rendering; ensure you
update the same loading state used by the skeleton component and keep the rest
of the fetch logic (fetchMember / setMember) unchanged.

309-333: ⚠️ Potential issue | 🟡 Minor

Add router to the handleExport dependency array.

The callback references router.replace but doesn't include router in its dependencies, which could cause stale closure issues.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/app/dashboard/members/`[userId]/page.tsx around lines 309 - 333, The
handleExport callback references router.replace but router isn't listed in the
useCallback dependencies, which can cause a stale closure; update the dependency
array for the handleExport hook to include router (e.g., useCallback(...,
[guildId, router])) so that router.replace is stable and the callback updates
when router changes.

357-359: ⚠️ Potential issue | 🟡 Minor

Replace array index key with stable semantic keys.

Using array indices as keys triggers lint warnings and can cause reconciliation issues. Use meaningful identifiers instead.

♻️ Suggested fix
-          {Array.from({ length: 4 }).map((_, i) => (
-            <Skeleton key={i} className="h-24 rounded-xl" />
+          {['messages', 'days-active', 'xp', 'reactions'].map((k) => (
+            <Skeleton key={k} className="h-24 rounded-xl" />
           ))}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/app/dashboard/members/`[userId]/page.tsx around lines 357 - 359, The
current Array.from({ length: 4 }).map((_, i) => <Skeleton key={i} .../>) uses
the loop index as a React key (in page.tsx), which causes lint warnings and
unstable reconciliation; replace the index with stable semantic identifiers by
mapping over a small explicit array of IDs (e.g. const placeholders =
['header','meta','activity','actions'] or similar) and render
placeholders.map(id => <Skeleton key={id} className="h-24 rounded-xl" />) so the
Skeleton components use stable, meaningful keys instead of the numeric index.

189-202: 🛠️ Refactor suggestion | 🟠 Major

Move member detail state and fetch logic into a Zustand store.

The component manages fetch lifecycle, response state, and mutations directly via local useState hooks instead of using the required fetch-on-demand Zustand store pattern. As per coding guidelines, "web/src/**/*.{tsx,ts}: Use Zustand store (zustand) for state management in React components; implement fetch-on-demand pattern in stores".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/app/dashboard/members/`[userId]/page.tsx around lines 189 - 202, This
component currently uses local useState (data/setData, loading/setLoading,
error/setError, xpAmount/xpReason/xpSubmitting/xpSuccess/xpError,
exporting/exportError) and inline fetch/mutation logic; move all member-detail
state and fetch-on-demand logic into a Zustand store that exposes selectors and
actions (e.g., memberDetailStore with getMemberDetail(memberId):
Promise<MemberDetailResponse>, state fields like member, loading, error,
xpAdjust(action) and exportMember(action)). Replace local useState usages with
useStore selectors for member, loading, error, xp status and exporting; call the
store's getMemberDetail from the component to trigger fetch-on-demand (if member
is null or stale) and invoke the store's xpAdjust and export actions instead of
local mutation code, then remove the local fetch/mutation implementations from
the component. Ensure MemberDetailResponse stays as the store's type and keep
xpAmount/xpReason only as transient controlled inputs in the component (not for
fetch lifecycle).

426-438: 🧹 Nitpick | 🔵 Trivial

Cache roleColorStyle result to avoid redundant function calls.

roleColorStyle(role.color) is called three times per role. Cache the result in a variable for minor efficiency and cleaner code.

♻️ Suggested fix
-              {data.roles.map((role) => (
-                <span
+              {data.roles.map((role) => {
+                const color = roleColorStyle(role.color);
+                return (
+                  <span
                     key={role.id}
                     className="inline-flex items-center rounded-full px-2.5 py-0.5 text-xs font-medium border"
                     style={{
-                      color: roleColorStyle(role.color),
-                      borderColor: `${roleColorStyle(role.color)}40`,
-                      backgroundColor: `${roleColorStyle(role.color)}15`,
+                      color,
+                      borderColor: `${color}40`,
+                      backgroundColor: `${color}15`,
                     }}
                   >
                     {role.name}
                   </span>
-              ))}
+                );
+              })}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/app/dashboard/members/`[userId]/page.tsx around lines 426 - 438,
Inside the data.roles.map loop, avoid calling roleColorStyle(role.color) three
times per role; compute const color = roleColorStyle(role.color) once (e.g., at
start of the map callback or inside the arrow function before JSX) and use color
for color, borderColor:`${color}40`, and backgroundColor:`${color}15` so the
markup uses the cached value; update references in the span JSX accordingly
(referencing roleColorStyle and the map callback for location).
src/api/routes/members.js (2)

184-198: ⚠️ Potential issue | 🟠 Major

Search is page-local, yielding incomplete results for large guilds.

The search filter is applied only to the single page returned by guild.members.list, so members matching the search on other pages won't be found. For accurate global search, accumulate matches across pages until reaching the requested limit.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/api/routes/members.js` around lines 184 - 198, The current implementation
applies the search filter only to the single page returned by guild.members.list
(using fetchOptions, limit, after), causing incomplete results for large guilds;
change it to page through guild.members.list (iterating with the cursor/after)
and accumulate matching members into memberList until you have collected the
requested limit or there are no more pages, applying the searchLower filter to
each page's members as you fetch them, and then return the accumulated results
(ensure you still respect the original limit and stop paging early once the
limit is reached).

258-282: ⚠️ Potential issue | 🟠 Major

Add name sort option to meet stated member-table requirements.

The PR objectives specify sorting by "name, messages, XP, warnings" but validSorts excludes 'name'. Add name sorting using locale-aware string comparison.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/api/routes/members.js` around lines 258 - 282, The member sorting
currently omits the "name" option: update the validSorts array to include 'name'
and add a corresponding branch in the enriched.sort switch so name comparison
uses a locale-aware string compare (e.g., a.name?.localeCompare(b.name || '',
undefined, { sensitivity: 'base' })) with proper null/undefined fallback; ensure
the returned value still respects the existing order === 'asc' ? aVal - bVal :
bVal - aVal flow by using the localeCompare result (or its numeric inversion)
for the 'name' case inside the enriched.sort block where sort is evaluated.
tests/modules/events-extra.test.js (1)

443-478: ⚠️ Potential issue | 🟠 Major

Ready-handler tests are missing outcome assertions.

These tests execute the ready callback but do not assert the behaviors claimed in the test names (starboard logging, model resolution). Without assertions on observable outcomes (e.g., logger calls, resolved config values), regressions can pass unnoticed.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/modules/events-extra.test.js` around lines 443 - 478, The tests under
registerReadyHandler execute the ready callback but lack assertions; update each
case to assert observable outcomes: mock and assert logger methods (e.g., a
mocked logger.info/error) when verifying "starboard info" for config.starboard,
and for the triage tests assert the resolved respondModel value is applied (by
inspecting the module/config state, a returned value, or a mock call that
receives the model) when calling registerReadyHandler; specifically augment the
tests that call registerReadyHandler(client, config, null) and then invoke
once.mock.calls[0][1]() to also assert logger calls or that the resolved model
equals 'gpt-4', 'custom-model', and 'explicit-model' respectively, using the
same mocked objects (client.once and any logger or triage consumer) so the tests
validate the claimed behaviors.
tests/modules/rateLimit.test.js (3)

626-885: ⚠️ Potential issue | 🟠 Major

Remove duplicated test suites — they re-run identical scenarios.

Lines 626-885 redefine suites already covered at lines 346-624 (handleRepeatOffender edge cases, trigger window reset, stopRateLimitCleanup, and singular minute in alert). This doubles test runtime and creates maintenance drift risk.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/modules/rateLimit.test.js` around lines 626 - 885, The file contains
duplicated describe blocks for the same scenarios (the suites titled
'checkRateLimit — handleRepeatOffender edge cases', 'checkRateLimit — trigger
window reset', 'stopRateLimitCleanup', and 'checkRateLimit — singular minute in
alert') which repeat tests already defined earlier; remove the later duplicated
describe blocks (the ones re-running tests that call checkRateLimit and
stopRateLimitCleanup, including the tests referencing member/timeouts/alertSend
and timer advances) so each scenario is only defined once and test
runtime/maintenance are reduced.

529-553: ⚠️ Potential issue | 🟡 Minor

Test does not verify the trigger counter was actually reset.

The test only asserts result.limited === true, which would pass regardless of whether the trigger counter reset occurred. To properly validate the reset logic, assert on the trigger count or verify subsequent behavior differs from the non-reset case.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/modules/rateLimit.test.js` around lines 529 - 553, The test only checks
result.limited but doesn't confirm the trigger counter was reset; update the
assertions after the second trigger block to verify the trigger counter was
reset to 1 (not 2) by asserting the trigger count returned from checkRateLimit
(e.g., result.triggerCount === 1) or by reading the rate-limit state exposed by
checkRateLimit/getRateLimitState for that message; ensure you reference the
checkRateLimit call that produced result and add an assertion like
expect(result.triggerCount).toBe(1) (or the equivalent property your function
returns) immediately after the final check in the second block.

582-624: ⚠️ Potential issue | 🟡 Minor

Test does not verify singular "minute" wording in the alert.

The test description claims to validate singular "minute" formatting, but it only checks that alertSend was called. Assert the embed text contains "1 minute" and not "1 minutes" to validate the formatting logic.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/modules/rateLimit.test.js` around lines 582 - 624, The test currently
only asserts that alertSend was called; update it to assert the alert's
message/embed includes the singular wording "1 minute". After the three
checkRateLimit calls, grab the first argument passed to alertSend (e.g. const
sent = alertSend.mock.calls[0][0]) and assert the stringified or relevant embed
field contains "1 minute" (expect(...).toContain('1 minute')) so the test
verifies singular vs plural formatting from checkRateLimit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/modules/rateLimit.test.js`:
- Around line 343-344: The import of stopRateLimitCleanup is mid-file; move the
ESM import statement for stopRateLimitCleanup (from
../../src/modules/rateLimit.js) up to the top import block with the other
rateLimit imports (where other imports from the same module appear) and remove
the duplicate import lines currently at lines 343-344 so all import declarations
are grouped at the file start.

In `@tests/sentry.init.test.js`:
- Around line 64-72: Replace the direct deletion of the environment variable
with Vitest's env stubbing API so the test teardown can restore state safely: in
the "should fall back to 0.1 when SENTRY_TRACES_RATE is not set" test, change
the line `delete process.env.SENTRY_TRACES_RATE` to use
`vi.stubEnv('SENTRY_TRACES_RATE', undefined)` (or an empty string) so that
`vi.unstubAllEnvs()` can correctly restore environment state; keep the rest of
the test (vi.stubEnv('SENTRY_DSN', ...), import('../src/sentry.js'), and the
assertion against initSpy) unchanged.

---

Duplicate comments:
In `@src/api/routes/members.js`:
- Around line 184-198: The current implementation applies the search filter only
to the single page returned by guild.members.list (using fetchOptions, limit,
after), causing incomplete results for large guilds; change it to page through
guild.members.list (iterating with the cursor/after) and accumulate matching
members into memberList until you have collected the requested limit or there
are no more pages, applying the searchLower filter to each page's members as you
fetch them, and then return the accumulated results (ensure you still respect
the original limit and stop paging early once the limit is reached).
- Around line 258-282: The member sorting currently omits the "name" option:
update the validSorts array to include 'name' and add a corresponding branch in
the enriched.sort switch so name comparison uses a locale-aware string compare
(e.g., a.name?.localeCompare(b.name || '', undefined, { sensitivity: 'base' }))
with proper null/undefined fallback; ensure the returned value still respects
the existing order === 'asc' ? aVal - bVal : bVal - aVal flow by using the
localeCompare result (or its numeric inversion) for the 'name' case inside the
enriched.sort block where sort is evaluated.

In `@tests/api/routes/members.test.js`:
- Around line 49-95: The shared mock fixtures (mockGuild, mockMember1,
mockMember2 and mockGuild.members with members.list/members.fetch) are mutated
across tests causing potential leakage; move their construction into a
beforeEach that creates fresh instances for mockMember1, mockMember2 and
mockGuild (including members.cache, members.list and members.fetch mocks) so
each test gets a new independent fixture, and update tests that currently call
mockResolvedValueOnce to operate on the per-test mocks instead of the shared
objects.

In `@tests/modules/events-extra.test.js`:
- Around line 443-478: The tests under registerReadyHandler execute the ready
callback but lack assertions; update each case to assert observable outcomes:
mock and assert logger methods (e.g., a mocked logger.info/error) when verifying
"starboard info" for config.starboard, and for the triage tests assert the
resolved respondModel value is applied (by inspecting the module/config state, a
returned value, or a mock call that receives the model) when calling
registerReadyHandler; specifically augment the tests that call
registerReadyHandler(client, config, null) and then invoke
once.mock.calls[0][1]() to also assert logger calls or that the resolved model
equals 'gpt-4', 'custom-model', and 'explicit-model' respectively, using the
same mocked objects (client.once and any logger or triage consumer) so the tests
validate the claimed behaviors.

In `@tests/modules/rateLimit.test.js`:
- Around line 626-885: The file contains duplicated describe blocks for the same
scenarios (the suites titled 'checkRateLimit — handleRepeatOffender edge cases',
'checkRateLimit — trigger window reset', 'stopRateLimitCleanup', and
'checkRateLimit — singular minute in alert') which repeat tests already defined
earlier; remove the later duplicated describe blocks (the ones re-running tests
that call checkRateLimit and stopRateLimitCleanup, including the tests
referencing member/timeouts/alertSend and timer advances) so each scenario is
only defined once and test runtime/maintenance are reduced.
- Around line 529-553: The test only checks result.limited but doesn't confirm
the trigger counter was reset; update the assertions after the second trigger
block to verify the trigger counter was reset to 1 (not 2) by asserting the
trigger count returned from checkRateLimit (e.g., result.triggerCount === 1) or
by reading the rate-limit state exposed by checkRateLimit/getRateLimitState for
that message; ensure you reference the checkRateLimit call that produced result
and add an assertion like expect(result.triggerCount).toBe(1) (or the equivalent
property your function returns) immediately after the final check in the second
block.
- Around line 582-624: The test currently only asserts that alertSend was
called; update it to assert the alert's message/embed includes the singular
wording "1 minute". After the three checkRateLimit calls, grab the first
argument passed to alertSend (e.g. const sent = alertSend.mock.calls[0][0]) and
assert the stringified or relevant embed field contains "1 minute"
(expect(...).toContain('1 minute')) so the test verifies singular vs plural
formatting from checkRateLimit.

In `@web/src/app/dashboard/members/`[userId]/page.tsx:
- Around line 204-205: The effect that fetches member data (useEffect) returns
early when guildId or userId is missing but never updates the loading state,
leaving loading true indefinitely; modify the effect in page.tsx to set loading
to false (or call setLoading(false)) and optionally set an error or null member
when guildId or userId is falsy before returning, so the skeleton stops
rendering; ensure you update the same loading state used by the skeleton
component and keep the rest of the fetch logic (fetchMember / setMember)
unchanged.
- Around line 309-333: The handleExport callback references router.replace but
router isn't listed in the useCallback dependencies, which can cause a stale
closure; update the dependency array for the handleExport hook to include router
(e.g., useCallback(..., [guildId, router])) so that router.replace is stable and
the callback updates when router changes.
- Around line 357-359: The current Array.from({ length: 4 }).map((_, i) =>
<Skeleton key={i} .../>) uses the loop index as a React key (in page.tsx), which
causes lint warnings and unstable reconciliation; replace the index with stable
semantic identifiers by mapping over a small explicit array of IDs (e.g. const
placeholders = ['header','meta','activity','actions'] or similar) and render
placeholders.map(id => <Skeleton key={id} className="h-24 rounded-xl" />) so the
Skeleton components use stable, meaningful keys instead of the numeric index.
- Around line 189-202: This component currently uses local useState
(data/setData, loading/setLoading, error/setError,
xpAmount/xpReason/xpSubmitting/xpSuccess/xpError, exporting/exportError) and
inline fetch/mutation logic; move all member-detail state and fetch-on-demand
logic into a Zustand store that exposes selectors and actions (e.g.,
memberDetailStore with getMemberDetail(memberId): Promise<MemberDetailResponse>,
state fields like member, loading, error, xpAdjust(action) and
exportMember(action)). Replace local useState usages with useStore selectors for
member, loading, error, xp status and exporting; call the store's
getMemberDetail from the component to trigger fetch-on-demand (if member is null
or stale) and invoke the store's xpAdjust and export actions instead of local
mutation code, then remove the local fetch/mutation implementations from the
component. Ensure MemberDetailResponse stays as the store's type and keep
xpAmount/xpReason only as transient controlled inputs in the component (not for
fetch lifecycle).
- Around line 426-438: Inside the data.roles.map loop, avoid calling
roleColorStyle(role.color) three times per role; compute const color =
roleColorStyle(role.color) once (e.g., at start of the map callback or inside
the arrow function before JSX) and use color for color,
borderColor:`${color}40`, and backgroundColor:`${color}15` so the markup uses
the cached value; update references in the span JSX accordingly (referencing
roleColorStyle and the map callback for location).

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a121cb0 and 01e0cbc.

📒 Files selected for processing (7)
  • src/api/index.js
  • src/api/routes/members.js
  • tests/api/routes/members.test.js
  • tests/modules/events-extra.test.js
  • tests/modules/rateLimit.test.js
  • tests/sentry.init.test.js
  • web/src/app/dashboard/members/[userId]/page.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: claude-review
  • GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (5)
**/*.js

📄 CodeRabbit inference engine (AGENTS.md)

**/*.js: Use ESM modules only — import/export, never require()
Always use node: protocol for Node.js builtin imports (e.g., import { readFileSync } from 'node:fs')

Files:

  • tests/modules/events-extra.test.js
  • src/api/index.js
  • tests/sentry.init.test.js
  • tests/api/routes/members.test.js
  • src/api/routes/members.js
  • tests/modules/rateLimit.test.js
**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{js,ts,tsx}: Always use semicolons
Use single quotes — enforced by Biome
Use 2-space indentation — enforced by Biome

Files:

  • tests/modules/events-extra.test.js
  • src/api/index.js
  • tests/sentry.init.test.js
  • tests/api/routes/members.test.js
  • src/api/routes/members.js
  • web/src/app/dashboard/members/[userId]/page.tsx
  • tests/modules/rateLimit.test.js
src/**/*.js

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.js: NEVER use console.log, console.warn, console.error, or any console.* method in src/ files — always use Winston logger instead: import { info, warn, error } from '../logger.js'
Pass structured metadata to Winston logger: info('Message processed', { userId, channelId })
Use custom error classes from src/utils/errors.js and always log errors with context before re-throwing
Use getConfig(guildId?) from src/modules/config.js to read config; use setConfigValue(path, value, guildId?) to update at runtime
Use safeSend() utility for outgoing Discord messages to sanitize mentions and enforce allowedMentions
Use splitMessage() utility for messages exceeding Discord's 2000-character limit
Add tests for all new code with mandatory 80% coverage threshold on statements, branches, functions, and lines; run pnpm test before every commit

Files:

  • src/api/index.js
  • src/api/routes/members.js
web/**/*.{tsx,ts}

📄 CodeRabbit inference engine (AGENTS.md)

Use next/image Image component with appropriate layout and sizing props in Next.js components

Files:

  • web/src/app/dashboard/members/[userId]/page.tsx
web/src/**/*.{tsx,ts}

📄 CodeRabbit inference engine (AGENTS.md)

Use Zustand store (zustand) for state management in React components; implement fetch-on-demand pattern in stores

Files:

  • web/src/app/dashboard/members/[userId]/page.tsx
🧠 Learnings (8)
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to src/**/*.js : Add tests for all new code with mandatory 80% coverage threshold on statements, branches, functions, and lines; run pnpm test before every commit

Applied to files:

  • tests/modules/events-extra.test.js
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to src/**/*.js : Use getConfig(guildId?) from src/modules/config.js to read config; use setConfigValue(path, value, guildId?) to update at runtime

Applied to files:

  • src/api/index.js
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to src/modules/moderation.js : Case numbering must be per-guild sequential, assigned atomically using COALESCE(MAX(case_number), 0) + 1 in a single INSERT statement

Applied to files:

  • src/api/routes/members.js
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to src/db.js : Database connection requires PostgreSQL (via pg driver with raw SQL, no ORM); the bot works without a database but config persistence requires PostgreSQL

Applied to files:

  • src/api/routes/members.js
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to web/src/**/*.{tsx,ts} : Use Zustand store (zustand) for state management in React components; implement fetch-on-demand pattern in stores

Applied to files:

  • web/src/app/dashboard/members/[userId]/page.tsx
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to src/**/*.js : NEVER use console.log, console.warn, console.error, or any console.* method in src/ files — always use Winston logger instead: import { info, warn, error } from '../logger.js'

Applied to files:

  • web/src/app/dashboard/members/[userId]/page.tsx
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to src/**/*.js : Use custom error classes from src/utils/errors.js and always log errors with context before re-throwing

Applied to files:

  • web/src/app/dashboard/members/[userId]/page.tsx
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to **/*.{js,ts,tsx} : Always use semicolons

Applied to files:

  • web/src/app/dashboard/members/[userId]/page.tsx
🧬 Code graph analysis (3)
tests/modules/events-extra.test.js (1)
src/modules/events.js (7)
  • registerReviewClaimHandler (363-393)
  • registerShowcaseButtonHandler (401-442)
  • registerShowcaseModalHandler (450-478)
  • registerChallengeButtonHandler (506-548)
  • registerReadyHandler (42-74)
  • registerMessageCreateHandler (103-252)
  • registerReactionHandlers (262-320)
src/api/index.js (8)
src/api/routes/members.js (1)
  • router (17-17)
src/api/routes/guilds.js (1)
  • router (21-21)
src/api/routes/auth.js (1)
  • router (15-15)
src/api/routes/moderation.js (1)
  • router (12-12)
src/api/routes/health.js (1)
  • router (28-28)
src/api/routes/config.js (1)
  • router (23-23)
src/api/routes/webhooks.js (1)
  • router (12-12)
src/api/middleware/auth.js (1)
  • requireAuth (36-70)
tests/modules/rateLimit.test.js (2)
src/modules/rateLimit.js (3)
  • checkRateLimit (146-230)
  • member (65-65)
  • stopRateLimitCleanup (271-276)
src/utils/modExempt.js (1)
  • member (24-24)
🪛 GitHub Check: CodeQL
src/api/index.js

[failure] 29-29: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.

src/api/routes/members.js

[failure] 55-55: Missing rate limiting
This route handler performs authorization, but is not rate-limited.


[failure] 57-154: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.


[failure] 168-168: Missing rate limiting
This route handler performs authorization, but is not rate-limited.


[failure] 168-301: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.


[failure] 311-311: Missing rate limiting
This route handler performs authorization, but is not rate-limited.


[failure] 313-414: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.


[failure] 428-428: Missing rate limiting
This route handler performs authorization, but is not rate-limited.


[failure] 430-477: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.


[failure] 490-490: Missing rate limiting
This route handler performs authorization, but is not rate-limited.


[failure] 492-579: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.

🔇 Additional comments (17)
tests/sentry.init.test.js (2)

12-19: LGTM!

The Sentry mock setup correctly intercepts init and related functions for testing configuration branches.


151-186: LGTM!

The sensitive key scrubbing test thoroughly validates that PII and secrets are removed from event.extra while preserving safe fields. Good coverage of the beforeSend security logic.

web/src/app/dashboard/members/[userId]/page.tsx (1)

83-86: LGTM!

The fallback color is now a valid hex value (#6b7280) that works correctly with hex alpha suffix concatenation (${color}40, ${color}15).

tests/modules/events-extra.test.js (3)

1-101: LGTM!

Comprehensive mock setup and imports provide good isolation for testing event handler edge cases. The mock implementations correctly stub dependencies while allowing verification of interactions.


503-514: LGTM!

Good coverage of error handling paths in registerMessageCreateHandler. Tests verify graceful degradation when AFK handler, rate limiter, link filter, and safeReply fail, ensuring the bot doesn't crash on transient errors.

Also applies to: 615-633


680-735: LGTM!

Thorough coverage of partial fetch edge cases in reaction handlers, including fetch failures, missing guild context, and bot reaction filtering. These tests validate resilience against Discord API edge conditions.

src/api/index.js (1)

27-32: LGTM!

Correct mount order ensures /:id/members/* routes are matched before the more generic guilds routes. Rate limiting is handled internally by membersRouter via its own membersRateLimit middleware, so no additional limiter is needed here. The CodeQL warning is a false positive since the router-level rate limiting in members.js covers all endpoints.

tests/api/routes/members.test.js (4)

134-165: LGTM!

Excellent test coverage for enriched member list response, verifying stats enrichment (messages_sent, xp, level, warning_count), default values for members without data, and response structure (total, nextAfter cursor).


474-481: LGTM!

Good test coverage for fractional XP rejection. This validates the Number.isInteger() check that prevents database errors from non-integer XP values.


659-679: LGTM!

Transaction rollback test properly verifies that ROLLBACK is called on failure and the client is released, ensuring no connection leaks during error conditions.


683-762: LGTM!

Comprehensive CSV escaping tests cover formula injection protection ('=CMD, '+injected), comma handling (double-quote wrapping), and quote/newline escaping. This validates the escapeCsv helper's security and correctness.

src/api/routes/members.js (6)

17-20: LGTM!

Rate limiter is properly defined with appropriate limits (120 req/15 min) for member management endpoints.


38-44: LGTM!

safeGetPool correctly wraps getPool() to return null on failure, enabling graceful 503 responses when the database is unavailable.


65-79: LGTM!

CSV export correctly paginates through all guild members in batches of 1000, addressing the previous concern about incomplete exports for large guilds.


496-507: LGTM!

XP validation correctly rejects non-finite, non-integer, zero, and out-of-bounds values with appropriate error messages. The Number.isInteger() check prevents database errors from fractional inputs.


516-552: LGTM!

XP adjustment uses a proper transaction with BEGIN/COMMIT/ROLLBACK for atomicity. The GREATEST(0, ...) ensures XP never goes negative, and level is recomputed from thresholds after adjustment.


589-601: LGTM!

escapeCsv correctly handles CSV injection by prefixing dangerous leading characters (=, +, -, @, \t, \r) with a single quote while preserving the original value. Commas, quotes, and newlines are properly escaped with RFC 4180-compliant quoting.

…tale responses

Add AbortController to cancel in-flight fetch requests when a new
request is triggered (e.g., from search/sort changes). Also add a
monotonic request ID counter to guard against out-of-order responses
overwriting newer state.

- Abort previous request on each new fetchMembers call
- Pass AbortSignal to fetch()
- Silently ignore AbortError exceptions
- Discard stale responses/errors via requestId guard
- Only clear loading state for the current (non-superseded) request
- Abort in-flight request on component unmount
…ltering

Replace client-side substring filtering (which only searched within the
current page of Discord results) with Discord's guild.members.search()
API for server-side search across all guild members.

- search param now triggers guild.members.search({ query, limit })
- Cursor pagination (nextAfter) is null during search (Discord search
  does not support cursor-based paging)
- filteredTotal reflects actual server-side search result count
- Sort still applies to the returned page (documented limitation —
  global sort would require DB-driven member indexing)
- Updated tests to verify search() is called and filteredTotal semantics
Replace the pattern of accumulating all guild members in one in-memory
Map before writing CSV.  Now each batch of 1000 members is fetched from
Discord, enriched from the DB, written to the response, and released
for GC — keeping peak memory at O(batch_size) instead of O(total_members).

- Move CSV header write before the batch loop
- Process and write each batch inline instead of collecting all first
- Remove userIds.length > 0 guards (batch loop guarantees non-empty)
- Track exportedCount incrementally
- Added streaming export test
Copy link

@claude claude bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 warnings (missing router in useCallback exhaustive-deps) and 4 nitpicks (Biome lint compliance). All issues have inline comments with suggested fixes. See the summary comment for the full table and a copy-paste fix prompt.

Copilot AI review requested due to automatic review settings February 28, 2026 03:51
@BillChirico BillChirico merged commit 512dbd7 into main Feb 28, 2026
10 of 12 checks passed
@BillChirico BillChirico deleted the feat/member-management branch February 28, 2026 03:54
const fetchOptions = { limit, after };
const discordPage = await guild.members.list(fetchOptions);
memberList = Array.from(discordPage.values());
const lastMember = Array.from(discordPage.values()).pop();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant array creation — memberList already contains the array from line 188

Suggested change
const lastMember = Array.from(discordPage.values()).pop();
const lastMember = memberList.at(-1);
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/api/routes/members.js
Line: 189

Comment:
Redundant array creation — `memberList` already contains the array from line 188

```suggestion
      const lastMember = memberList.at(-1);
```

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +46 to +48
// Debounce search
const searchTimerRef = useRef<ReturnType<typeof setTimeout>>(undefined);
const [debouncedSearch, setDebouncedSearch] = useState('');
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

searchTimerRef is typed as useRef<ReturnType<typeof setTimeout>> but initialized with undefined. With strict: true this is a type error because undefined isn’t assignable to the timeout handle type. Use useRef<ReturnType<typeof setTimeout> | null>(null) (or | undefined) and guard accordingly when calling clearTimeout/assigning.

Copilot uses AI. Check for mistakes.
Comment on lines +27 to 32
// Member management routes — require API secret or OAuth2 JWT
// (mounted before guilds to handle /:id/members/* before the basic guilds endpoint)
router.use('/guilds', requireAuth(), membersRouter);

// Guild routes — require API secret or OAuth2 JWT
router.use('/guilds', requireAuth(), guildsRouter);
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

requireAuth() is mounted twice under /guilds (once for membersRouter, again for guildsRouter). For non-member guild routes this runs the auth middleware twice, which is unnecessary work and can have unintended side effects if auth ever mutates request state. Consider mounting requireAuth() once and attaching both routers under it (or mounting membersRouter inside guildsRouter before other routes).

Copilot uses AI. Check for mistakes.
Comment on lines +175 to +190
// Fetch members — use Discord server-side search when a query is provided
// (searches all guild members by username/nickname prefix), otherwise use
// cursor-based listing. Sort is applied after enrichment and is scoped to
// the returned page; it does NOT globally sort all guild members.
let memberList;
let paginationCursor = null;
if (search) {
const searchResults = await guild.members.search({ query: search, limit });
memberList = Array.from(searchResults.values());
// Discord search does not support cursor pagination
} else {
const fetchOptions = { limit, after };
const discordPage = await guild.members.list(fetchOptions);
memberList = Array.from(discordPage.values());
const lastMember = Array.from(discordPage.values()).pop();
paginationCursor = lastMember ? lastMember.id : null;
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The API combines cursor pagination (after/nextAfter) with post-fetch sorting. Because nextAfter is derived from Discord’s cursor order, but members is then sorted (e.g., by XP/messages), clicking “Load More” will append a new page sorted independently, producing an overall list that is not actually sorted by the selected column. To avoid incorrect ordering, either implement keyset pagination aligned with the chosen sort field, or disable/omit nextAfter when sort is not the native Discord order (or rework the UI to clarify that sorting only applies within the current page).

Copilot uses AI. Check for mistakes.
Comment on lines +584 to +586
// Prevent CSV formula injection — prefix dangerous leading chars
const formulaChars = ['=', '+', '-', '@', '\t', '\r'];
if (str.length > 0 && formulaChars.includes(str[0])) {
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

escapeCsv only checks the first character for formula injection. Spreadsheet apps can still treat values like ' =1+1' (leading spaces) as formulas. Consider escaping when the string begins with optional whitespace followed by a dangerous character (e.g., /^\s*[=+\-@]/) and include other common prefixes like \uFEFF if relevant.

Suggested change
// Prevent CSV formula injection — prefix dangerous leading chars
const formulaChars = ['=', '+', '-', '@', '\t', '\r'];
if (str.length > 0 && formulaChars.includes(str[0])) {
// Prevent CSV formula injection — prefix values that begin (optionally after BOM/whitespace)
// with characters that spreadsheets may interpret as formulas.
// This protects against inputs like " =1+1", "\t-2", or "\uFEFF@cmd".
const formulaPattern = /^[\uFEFF\s]*[=+\-@\t\r]/;
if (formulaPattern.test(str)) {

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Web dashboard: member management — profiles, stats, reputation, and memory

2 participants